CHANGES-REQUIRED — correctness gate

Head reviewed: 6e2b6730a772f6fc92b39cd16c8273a666baed0b (resolved from the remote, not from the dispatch).

Recorded as a --comment review: a verdict event from the PR author's identity is a 422 for both APPROVE and REQUEST_CHANGES. The four open inline threads are what bar the merge, via required_conversation_resolution.

What I verified myself, by mutation, in my own worktree

Baseline at head: 968 passed, 0 failed.

The revert-proofs the PR claims are real. The gap is one level up: they protect the helpers, not the production call path.

The ceiling incident — verified, with one correction

RELAY_MAX_WAIT is present and correct at head (const module_transport.rs:564, the check at :624 before any further sleep, three tests). I traced the history independently: the check was removed in 27c97cd and restored in 203661c. Nothing else from that batch is missing — the only other deletions in 27c97cd are the intended descriptor_with_escalation → descriptor_via_rounds refactor, and all three RelayWaitEnd endings survive in both code and tests.

One correction to the write-up: 2a27fd3 is not an ancestor of this branch. The branch was rewritten; its on-branch twin is 27c97cd. The narrative is otherwise accurate, but a reader chasing 2a27fd3 will not find it in the history.

Verdict on the four constants

• 3 s grace — sound. Comfortably inside the 5 s first rung, so the small-capsule case stays byte-for-byte one round trip.
• 10 s poll — sound, with a doc nit: it is described as "one small round trip", but each poll is round(true) = a re-dial plus the full 65 s ladder. Cheap for a hop that answers Pending promptly; a hop that goes silent costs a 65 s ladder before being classed Abandoned. Non-gating.
• 60 s stall — sound. Correctly sized above the poll interval, and the monotonic-max best makes downward lying useless (it self-stalls at 60 s).
• 30 min ceiling — right instrument, wrong scope. See thread 2: it bounds one peer, while dig-download's documented worst case is 3 × holders × the transport's per-ask timeout, and this PR raises that term from 65 s to 30 min.

Also confirmed

• NC-12 core claim holds. staged_bytes is read only for best / last_advance and one tracing::debug!. It never gates acceptance, and the capsule still clears the chain anchor + merkle verification in warm_claiming.
• §2.4b spot-checked and the lane's claim is correct. dig-nat 0.20.0 → dig-tls ^0.4; dig-peer 0.12.0 → chia-protocol ^0.36 + dig-tls ^0.4; dig-dht 0.12.1 → dig-nat ^0.20, so it is blocked transitively too. dig-peer-selector 0.9.1 is already inside the declared caret. Nothing else is takeable without the chia-0.36 cascade (#3152). The two corrected Cargo.toml blocker comments are an improvement and match the index.
• capsule_warm_locator_tests — the set-based assertion keeps both halves: an empty set still catches a pull that asked nobody, and a second id still catches reaching past the pool. Correct call, correctly reasoned.
• #332 / dig-download 0.19.2 composition — the StagingDisposition justification holds. for_failure is the only producer of Discard, erase() is the only path that clears the checkpoint, and I found no second eraser.

Not blocking

.github/** is untouched, so there is no workflow collision with #336 — but Cargo.toml, Cargo.lock and SPEC.md all overlap. Thread 5, self-resolved.

Four threads open. I will re-verify and resolve each against the new head.

CHANGES-REQUIRED — scoped re-gate at 04494e09551daa00b0fdd2fe7604630270f225b9

(Independent fresh-context pass over the 6e2b673 -> 04494e0 delta. One gating finding, posted as an inline thread; everything else I was asked to check verified clean.)

Finding 1 (unification bypass) — VERIFIED, by reproduction

I reproduced the prior gate's bypass in my own worktree: replaced get_module_info's body with a direct self.ask_descriptor_once(..., false) and a match on the answer, deleting the ladder, the escalation and the relay wait from the shipped path.

Both production-path tests FAIL, and both fail bounded:

the_production_get_module_info_climbs_the_whole_ladder ... FAILED
  panicked at module_transport.rs:1649: `get_module_info` must be BOUNDED by the ladder ...: Elapsed(())
a_cold_first_invocation_escalates_through_the_production_transport ... FAILED
  assertion `left == right` failed: left: 1, right: 2

Restored, clean head is 25 passed; 0 failed for the module. So the tokio::time::timeout(ladder * 2, ...) wrapper does its job: the regression reports a red assertion in 0.01s, not a stuck job. The second test has no timer at all — its locator answers immediately and its dial path is synchronous-failing — so it is not hang-shaped in the first place. Both hang-shaped tests are bounded.

My own attempt at a NEW bypass:

• descriptor_ask is a private mod and ask_within_deadlines is private to it. The only item the parent can reach that yields a descriptor is descriptor_for_holder, and it is pub(super) — nothing outside module_transport can reach any of it. No pub(crate), no re-export, no trait impl, no #[allow] opening a side door.
• descriptor_over has exactly one caller (ask_descriptor_once), which has exactly one caller (the closure in get_module_info). Grepped crate-wide: no other call site.
• The honest caveat: descriptor_over and ask_descriptor_once are private to the FILE, so a call site inside module_transport can still be written to skip the policy — which is precisely where the original bypass was written. So the defect is not compiler-unexpressible; it is test-detected, reliably, which I measured. I am satisfied with that, and the claim in the module doc is a shade stronger than what the privacy boundary alone delivers.
• CountingLocator genuinely pins the CALL SITE, not a helper: the counter lives in find_providers, which is reached only via connect -> dial_targets -> discovered_candidates, once per dial, and a dial happens once per descriptor round. No helper increments it. Two rounds are producible only by the escalation, which only descriptor_for_holder implements.

Finding 2 (per-pull budget) — the arithmetic does NOT close. See the inline thread.

The within-one-pull arithmetic is right: remaining() is passed down as the ceiling, relay_waited is measured from immediately before wait_for_relayed_descriptor and is zero on every non-relaying branch, charge saturates, and the ceiling is checked before any further poll — so holders x 30 min no longer multiplies within a pull, and one honest hop can still spend the whole allowance.

What does not close is the ledger's LIFETIME. It is a field of a NatModuleTransport built once at node wiring (download.rs:2242), with no reset, no TTL and no clear on pull start or completion. So the key is the capsule for the life of the daemon, not the pull — and a capsule whose first honest pull spent the budget can never use the relay path again until restart, with the refusal attributed by name to a blameless peer. Detail, required fix, what the fix must not do, and the test it needs: the inline thread on module_transport.rs:329.

the_relay_budget_is_spent_per_capsule_and_saturates is a good test of what it covers, and it is structurally blind to this: it never models a second pull, so it passes identically with or without a reset.

Findings 3 and 4 — clear

Starvation is recorded at both ends and a_relayed_warm_and_a_local_warm_compete_for_the_same_slots carries the control arm (the local warm succeeds once a slot frees), so "refused" is not satisfied by a warm that never succeeds. Orphaned doc restored.

Rebase — four values read at HEAD, not from the diff

The SPEC §11.1b addition says what it was claimed to say — verified-complete is the asset's upload state (state == "uploaded", vs "starting"), not the presence of a row.

Checks

check-merge-preconditions.sh asserts all five required contexts present and SUCCESS by name (Lint commit messages, Check version increment, Rustfmt, Clippy, Test + coverage). Its non-zero exit is the DRAFT flag only. build .msi is pending and non-required; not treated as blocking.

Not re-verified per the scoped brief: no-wait, both #332 revert-proofs, the NC-12 core claim, the §2.4b deferral behind #3152, the capsule_warm_locator_tests set assertion, the StagingDisposition justification.

Verdict recorded as a comment review because a same-identity PR 422s any verdict-bearing event. The open inline thread is what bars the merge.

CHANGES-REQUIRED — scoped re-gate of 04494e0 -> 26c11bd

Head read: 26c11bdc2c4fdf877e650f2c1a3497c4269d9afb, resolved from the remote rather than taken from the dispatch. Base 5fdcf909. Scope: the two fixes only, per the re-gate brief.

One gating finding, posted inline on module_reshare.rs:802. Everything else in the delta verified and clears.

Fix 1 — budget lifetime: right shape, one uncovered exit path

• Called on every returning path — verified. All four public entry points funnel into warm_with_config; one download call site, one pull_finished immediately after it; the three earlier returns sit above the download and have nothing to release.
• Not called when the pull unwinds. See the inline finding: a panic inside download skips it, the tier-0 loop catches that panic by design (catch_iteration, a real catch_unwind), and the leaked entry reinstates both the permanent relay-ineligibility and the peer-named attribution the amended SPEC now forbids. A drop guard is strictly stronger and its absence matters.
• Same object under both roles — verified. download.rs:2250 is the only non-test CapsuleWarmer::new, and it hands Arc::clone(&transport) as ModuleTransport and transport as PullLifecycle — one allocation, one ledger. No second transport is constructed on the production path.
• The TTL rejection holds, premise checked. charge runs only after descriptor_for_holder returns (module_transport.rs:1512-1514), so nothing is recorded during a wait and a live entry is genuinely indistinguishable from an idle one for up to the whole ceiling. The argument is not merely plausible — its premise is how the code works.
• Eviction cannot expire a live pull — discharged with a bound. WarmRegistry admits DEFAULT_MAX_CONCURRENT_WARMS = 4 distinct generations node-wide, relayed and local sharing the one cap, so at most four entries are ever live. charge evicts FIFO from the front only when inserting a new key at 1024 entries, and a live entry is always among the newest, so reaching one would require the 1023 older entries to be live too. Unreachable.

The three tests — re-run here, and they discriminate

Baseline green in 0.04s before mutating. The third test's failing-pull half is load-bearing: under the mutation it is the failed-pull assertion at module_reshare.rs:1934 that fires first, so a fixture covering only the success path would not have caught it. Run in an isolated worktree, alone, per the os error 32 warning — no lock failure reproduced.

One process note on the re-run: filtering with cargo test ... --lib budget matches 27 tests and none of them is releasing_one_pull_does_not_refund_another, whose name contains no such substring. That filter reports ok while the test under examination never runs. Both budget-lifetime tests have to be named explicitly.

Fix 2 — the over-claim: correct

unbypassable no longer appears anywhere in the crate. The one remaining occurrence is SPEC.md:5972, a pre-existing and unrelated claim about an announcement backstop, byte-identical at 04494e0 and untouched here.

The replacement wording is accurate: Rust privacy is module-scoped, so the boundary is against the rest of the crate and not against an in-file edit, and the two tests it now leans on both exist and drive the real method — the_production_get_module_info_climbs_the_whole_ladder (module_transport.rs:1719) and a_cold_first_invocation_escalates_through_the_production_transport (:1784). The load-bearing claim is stated as detection rather than prevention, which is what is true.

Rebase values, read by value at HEAD

Non-gating, resolved by me — eviction rationale is now stale

RelayWaitBudget's doc reads "evicting the oldest merely refreshes the budget of a capsule this node stopped working on long ago". After this fix, a capsule stopped working on long ago is no longer in the ledger at all — every entry is a live pull. The bound above makes it harmless, but the sentence argues for safety from a premise the fix removed. Worth one line when the file is next open; not a blocker, and not a reason for a round.

Scale note

This is a strict improvement on what is shipped today, where the entry leaks on every pull rather than only on a panicking one. The verdict is CHANGES-REQUIRED because the code and the normative SPEC now both assert a MUST the code does not hold, and the remedy is a local, roughly ten-line RAII change in the same file.

Shared-checkout disclosure

Read-only in the primary dig-node checkout: git fetch, git show, git diff, git grep, and git worktree add / prune. All probing happened in C:/tmp/worktrees/gate334, which is now removed. The primary working tree was not modified by me; the staged entries visible there pre-existed and belong to another lane.

VERDICT: PASS — re-gate round 3 (scoped)

Head reviewed: 71f0de49656b680fa565577218e662f26a0875bb (base 5fdcf909; delta from 26c11bd = 2 files, +151/-8).
Recorded as a --comment review: a same-identity PR 422s any verdict-bearing event, so this comment IS the verdict.

Scope was the single PullBoundary fix and its test. Everything cleared in rounds 1-2 was not re-verified.

1. The guard is a strict superset, not a re-shuffle

module_reshare.rs:844-847. Exactly ONE pull_finished call site existed before (the plain statement after the
.await); exactly ONE mechanism exists now (Drop for PullBoundary), and the drop point is the SAME position —
the block closes immediately after the await, with nothing between it and the old statement's line.

• Every returning path (Ok, Err) and the unwinding path now go through one mechanism.
• Nothing that previously reported now does not. Paths that return BEFORE the download (no chain anchor, warm-cap
  skip, non-canonical id) did not report before either, and correctly still do not — no relay budget has been
  charged at that point.
• Strictly better on a third path the old code missed: future CANCELLATION. If the warm future is dropped
  mid-download the guard still runs; the plain statement did not.

2. Scope is right

Verified the code between the block and the old call site: StagingDisposition::for_failure / apply,
promote_into_cache, pull.erase(), announce_inventory(), and the tracing calls. None of them touches
ModuleTransport or charges RelayWaitBudget under (store_id, root), so releasing at block-end rather than at
end of function relies on nothing still-live.

3. The guard cannot panic, and cannot be a silent no-op

Drop::drop -> PullLifecycle::pull_finished -> NatModuleTransport::release (module_transport.rs:379-393).
That path is:

• Poison-tolerant on both mutexes — lock().unwrap_or_else(|p| p.into_inner()), so a panic that poisoned the
  budget mid-charge cannot make the release panic during a subsequent unwind. No unwrap, no expect, no ?.
• Lock-order consistent with charge (spent then order in both), so no inversion deadlock.
• No I/O, no logging, no fallible allocation beyond format! — nothing that can panic during an unwind and
  turn a survivable panic into a process abort.

pull_lifecycle is Arc<dyn PullLifecycle>, never an Option, so as_ref() is infallible and the guard is never
absent. The only no-op impl, NoPullState, is #[cfg(test)] and its doc comment says why — production has one
wiring and it is NatModuleTransport.

4. No second download call site

At 71f0de4 there is exactly one production ModuleDownloader::download in the crate
(module_reshare.rs:846), inside the boundary. The other .download( hit in the tree is
download.rs:2044, a different type on the content-id path that does not carry the relay budget. The delta added
none.

The revert-proof, re-run independently — passed-count read explicitly

Own detached worktree at 71f0de4 (C:/tmp/worktrees/gate-334-rp); the shared checkout and the lane's own
dn-3128 worktree were never touched.

Baseline: test result: ok. 24 passed; 0 failed — 24, not 23; the module has 24 including the new test.

Mutation — restored the plain statement, grep-confirmed BEFORE running: zero occurrences of
PullBoundary::new, one occurrence of let pulled = downloader.download(...) at line 844. No cargo fmt was run,
so the mutation could not be silently reformatted away.

Mutated: test result: FAILED. 23 passed; 1 failed — and the one failure is exactly
a_pull_that_panics_still_reports_its_end, on the intended assertion, with left: [] against the expected
[(store, root)] pair.

Three things this establishes, beyond "it went red":

• The control is real and fired FIRST. outcome.is_err() passed under the mutation, so the pull genuinely
  panicked and was genuinely caught. "Reported its end" could not have been satisfied by a pull that never panicked.
• The test names the PROPERTY, not an outcome. It failed with an ABSENT report, which is a placement failure.
  The nearest wrong implementation — report after the await — is precisely what was restored, and it does not pass.
• No other test depends on the guard; the other 23 stayed green, so the fix is not load-bearing anywhere it
  was not meant to be.

Worktree restored to clean (git status --porcelain empty) after the run.

Non-ASCII sweep of the delta

Swept every ADDED line across the full PR delta (5fdcf909..71f0de4), not just the two files this round touched.
Surviving non-ASCII, complete census: 80 em-dash, 6 multiplication-sign, 4 section-sign. All deliberate prose.
Zero CJK / kana / hangul — the two stray characters are gone and nothing similar replaced them.

Stale eviction prose

module_transport.rs:330-340 now states the true mechanism: entries released per pull via PullLifecycle,
WarmRegistry capping live warms node-wide, and the 1024 FIFO cap as a bound on a FUTURE leak rather than the
working mechanism. Confirmed DEFAULT_MAX_CONCURRENT_WARMS = 4 at module_reshare.rs:64, so the reasoning it
rests on holds by value.

Rebase values at HEAD, by value

• root Cargo.toml version = 0.146.0
• crates/dig-node-core/Cargo.toml version = 0.55.0
• Cargo.lock dig-download = 0.19.2 (crates.io registry source)
• dig_ecosystem#3152 named in the manifest comments — 4 occurrences across root + core, each explaining a
  deliberate hold on the chia-0.36 line

Diff is 14 files, +1999/-271, as stated.

Non-gating notes (raised and self-resolved, not blocking)

1. Both new assertion messages carry an interior run of spaces from a wrapped string literal, so the failure text
   reads with a gap. Cosmetic, appears only on failure output. Not worth a round.
2. PullBoundary borrows rather than owning, so it cannot outlive the warmer — correct today, and worth
   remembering if the pull is ever moved into a spawned task, where an owned Arc would be needed instead.

No gating findings. No threads opened.